Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature: control queue concurrency #849

Closed

Conversation

kanzihuang
Copy link
Contributor

// Config specifies the server's background-task processing behavior.
type Config struct {
// Maximum number of concurrent tasks of a queue.
//
// If set to a zero or not set, NewServer will not limit concurrency of the queue.
QueueConcurrency map[string]int
}

@kanzihuang kanzihuang force-pushed the feat/queue-concurrency-control branch from 3f14ff3 to a4802c1 Compare March 20, 2024 08:16
@kanzihuang kanzihuang force-pushed the feat/queue-concurrency-control branch 4 times, most recently from 652494f to 1c3bdbc Compare March 21, 2024 01:40
Copy link

codecov bot commented Mar 21, 2024

Codecov Report

Attention: Patch coverage is 91.66667% with 3 lines in your changes are missing coverage. Please review.

Project coverage is 68.85%. Comparing base (8df0bfa) to head (07d2722).
Report is 2 commits behind head on master.

❗ Current head 07d2722 differs from pull request most recent head 8f93c46. Consider uploading reports for the commit 8f93c46 to get more accurate results

Files Patch % Lines
internal/rdb/rdb.go 66.66% 3 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #849      +/-   ##
==========================================
+ Coverage   68.41%   68.85%   +0.43%     
==========================================
  Files          27       28       +1     
  Lines        3841     3859      +18     
==========================================
+ Hits         2628     2657      +29     
+ Misses        929      915      -14     
- Partials      284      287       +3     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Collaborator

@kamikazechaser kamikazechaser left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall I think the approach is fine by checking the count of the $queue:lease set. Since this is a major feature and introduces couple of internal changes, I'll let @hibiken comment on the changes first before we proceed to fix any small issues (e.g. tests/coverage).

example_test.go Outdated Show resolved Hide resolved
client_test.go Show resolved Hide resolved
processor.go Show resolved Hide resolved
@kanzihuang kanzihuang force-pushed the feat/queue-concurrency-control branch 2 times, most recently from ec15161 to 07d2722 Compare March 21, 2024 14:37
@shuqingzai
Copy link

I hope this PR can be merged quickly. The current priority queue cannot accurately control the number of consumers in different queues. ☺️

@kanzihuang kanzihuang force-pushed the feat/queue-concurrency-control branch from 07d2722 to 8f93c46 Compare April 30, 2024 22:30
@kanzihuang kanzihuang closed this May 13, 2024
@kanzihuang kanzihuang deleted the feat/queue-concurrency-control branch May 13, 2024 08:21
@shuqingzai
Copy link

@kanzihuang @kamikazechaser This PR is closed. Is there any chance for it to be merged into Master? This is a very important feature.

See #850 #852 #856 #903

@kamikazechaser
Copy link
Collaborator

kamikazechaser commented Nov 10, 2024

#879 is the latest PR around this. I'm yet to get a response from the author.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants